Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

url encode hashes in ref names #299

Merged
merged 1 commit into from Oct 25, 2016
Merged

url encode hashes in ref names #299

merged 1 commit into from Oct 25, 2016

Conversation

bsheats
Copy link

@bsheats bsheats commented Oct 21, 2016

Without this fix, the following code:

GHRepository repo = gh.getRepository("iown/test");
String branch = "heads/bsheats/#107-test-branch";
ref = repo.getRef(branch);

will fail with this exception:

Exception in thread "main" org.kohsuke.github.HttpException: Server returned HTTP response code: 200, message: 'OK' for URL: https://api.github.com/repos/iown/test/git/refs/heads/bsheats/#107-test-branch
    at org.kohsuke.github.Requester.parse(Requester.java:540)
    at org.kohsuke.github.Requester._to(Requester.java:251)
    at org.kohsuke.github.Requester.to(Requester.java:213)
    at org.kohsuke.github.GHRepository.getRef(GHRepository.java:770)
    at Foo.main(Foo.java:22)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)
Caused by: java.io.IOException: Failed to deserialize [{"ref":"refs/heads/bsheats/test","url":"https://api.github.com/repos/iown/test/git/refs/heads/bsheats/test","object":{"sha":"7c4a0777bdbd76442ee98e54daee808a5ca0e211","type":"commit","url":"https://api.github.com/repos/iown/test/git/commits/7c4a0777bdbd76442ee98e54daee808a5ca0e211"}},{"ref":"refs/heads/bsheats/#107-test-branch","url":"https://api.github.com/repos/iown/test/git/refs/heads/bsheats/%23107-test-branch","object":{"sha":"fca609f46dbbee95170e8a1dc283329ea1c768f6","type":"commit","url":"https://api.github.com/repos/iown/test/git/commits/fca609f46dbbee95170e8a1dc283329ea1c768f6"}}]
    at org.kohsuke.github.Requester.parse(Requester.java:530)
    ... 9 more
Caused by: com.fasterxml.jackson.databind.JsonMappingException: Can not deserialize instance of org.kohsuke.github.GHRef out of START_ARRAY token
 at [Source: java.io.StringReader@e50a6f6; line: 1, column: 1]
    at com.fasterxml.jackson.databind.JsonMappingException.from(JsonMappingException.java:164)
    at com.fasterxml.jackson.databind.DeserializationContext.mappingException(DeserializationContext.java:575)
    at com.fasterxml.jackson.databind.DeserializationContext.mappingException(DeserializationContext.java:569)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromArray(BeanDeserializerBase.java:1121)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:148)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:123)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:2888)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2034)
    at org.kohsuke.github.Requester.parse(Requester.java:528)
    ... 9 more

@kohsuke
Copy link
Collaborator

kohsuke commented Oct 24, 2016

It shouldn't be just '#' that we'd be replacing, so we need to be using like URLEncoder.encode() I think

@bsheats
Copy link
Author

bsheats commented Oct 24, 2016

Yeah, I tried that originally, but you actually don't want to url encode the slashes, so I think the only special character is the hash. GitHub's api is a bit peculiar because branches with slashes are appended directly to the api request (e.g. branch 'bsheats/test' maps to https://api.github.com/repos/iown/test/git/refs/heads/bsheats/test)

@kohsuke
Copy link
Collaborator

kohsuke commented Oct 25, 2016

OK, let's start this change and let whoever needs something more proper to improve it later.

@kohsuke kohsuke merged commit 5334cb8 into hub4j:master Oct 25, 2016
kohsuke added a commit that referenced this pull request Oct 25, 2016
@kohsuke
Merge pull request #299
89aac45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bsheats @kohsuke